RAI-22855 fix issues with (u)int128 within value types #102
Conversation
Wiz Scan Summary
|
|
Thanks for this PR @wienleung 🙏 I want to just approve so we can move on with our lives, but I'm wondering why the JS SDK is able to get away with code for debugging value types that appears to be so much simpler: https://github.com/RelationalAI/rai-sdk-javascript/blob/3457821ebdd6d6a01ec328b39b37226bd9dff713/src/results/resultUtils.ts#L252-L273 Seems like they're factored differently — in the JS SDK, I'd love to simplify this, but it seems like a bigger refactor than we have bandwidth for right now, so maybe let's just go for it. @NHDaly does this sound right to you? |
|
Honestly IDK. Somehow the Go version has to deal with arrays of lists and structs from the unmarshalled Arrow types and struct, and that doesn't seem to be the case for the JS version (at least from a quick look), which seems to happily yank values out of the Arrow table and convert them thru that function you linked |
| @@ -431,6 +448,58 @@ func (c listItemColumn[T]) Value(rnum int) any { | |||
| return c.Item(rnum) | |||
| } | |||
|
|
|||
| // Represents several sub-columns of a `listColumn` that represent one column for a composite type (e.g. int128) | |||
| type listSliceColumn[T any] struct { | |||
There was a problem hiding this comment.
is this what we were calling compositeColumn on our call? I think I liked that name a bit better
There was a problem hiding this comment.
I didn't call it that because this isn't the only way values made by composing primitive types together are stored as columns. Not a good general preference, but here I think we should be clear about how the struct works vs what user-level functionality it's intended for, given the complexity of all the various "column" transformations going on
Handle the case where there are composite values (i.e. large values represented by multiple arrow columns) within value types.